Skip to content

Improve doc comment for GIDSignInButtonStyle#204

Open
kamimi01 wants to merge 2 commits intogoogle:mainfrom
kamimi01:feature/fix-document-comments
Open

Improve doc comment for GIDSignInButtonStyle#204
kamimi01 wants to merge 2 commits intogoogle:mainfrom
kamimi01:feature/fix-document-comments

Conversation

@kamimi01
Copy link

I worked on #203

I think to be customizable the display text is better than adding document comments like this, so this is a workaround.

@petea petea linked an issue Aug 22, 2022 that may be closed by this pull request
@petea petea changed the title fix: Document comment for GIDSignInButtonStyle Improve doc comment for GIDSignInButtonStyle Aug 22, 2022
@kamimi01
Copy link
Author

@petea Hi team,
Thank you for checking my pull request. As I mentioned, I think to be customizable display text is better than adding doc comments. Do you have a plan to be able to customize it? If you don't, would you mind telling me a reason?
Thank you.

/// - kGIDSignInButtonStyleStandard: 230 x 48
/// - kGIDSignInButtonStyleWide: 312 x 48
/// - kGIDSignInButtonStyleIconOnly: 48 x 48 (no text, fixed size)
/// - kGIDSignInButtonStyleStandard: 230 x 48 (Display text is "Sign in")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future-proofing this a bit :)

Suggested change
/// - kGIDSignInButtonStyleStandard: 230 x 48 (Display text is "Sign in")
/// - kGIDSignInButtonStyleStandard: 230 x 48 (Example display text: "Sign in")

/// - kGIDSignInButtonStyleWide: 312 x 48
/// - kGIDSignInButtonStyleIconOnly: 48 x 48 (no text, fixed size)
/// - kGIDSignInButtonStyleStandard: 230 x 48 (Display text is "Sign in")
/// - kGIDSignInButtonStyleWide: 312 x 48 (Display text is "Sign in with Google")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// - kGIDSignInButtonStyleWide: 312 x 48 (Display text is "Sign in with Google")
/// - kGIDSignInButtonStyleWide: 312 x 48 (Example display text: "Sign in with Google")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix document comment of GoogleSignIn (product "GoogleSignIn")

3 participants